disable type validity assertions for better error messages #4434
Conversation
|
Did you have a specific reason for putting the logic here instead of using |
|
No, both seemed about equally valid to me and this approach keeps the concern local to Miri. It also means const-eval code still gets the assertion even when running in Miri.
But I can make this cfg(miri) in libs if you prefer *shrug*
|
| })); | ||
| } | ||
|
|
||
| // Overwrite some core engine intrinsics that are used to catch UB -- we have better |
There was a problem hiding this comment.
Shouldn't we remove them from the core engine and add them to the ctfe intrinsic handler if miri doesn't want them?
There was a problem hiding this comment.
That's also an option, yeah. Maybe that's more clear.
So what should it be:
- do this in the library?
- or do it by neutering the intrinsic in Miri?
There was a problem hiding this comment.
I like this variant as it keeps the checks in miri const eval as you said
There was a problem hiding this comment.
Oh, because we don't do validation in normal const eval? So if we removed these checks for all interpreters, we'd miss even more UB in const eval?
There was a problem hiding this comment.
Oh, because we don't do validation in normal const eval? So if we removed these checks for all interpreters, we'd miss even more UB in const eval?
Exactly.
|
Closing in favor of rust-lang/rust#143327. |
3 participants
Miri has pretty nice errors for type validity violations, printing which field in the type the problem occurs at and so on.
However, we don't see these errors when using e.g.
mem::zeroedas that usesassert_zero_validto bail out before Miri can detect the UB.Similar to what we did with @saethlin's UB checks, I think we should disable such language UB checks in Miri so that we can get better error messages. If we go for this we should probably say this in the intrinsic docs as well so that people don't think they can rely on these intrinsics catching anything.
@rust-lang/miri what do you think?